[cueweb] Add job dependency graph: inline panel + Cuetopia toggle#2377
Conversation
|
|
📝 WalkthroughWalkthroughThis PR adds a read-only job dependency graph visualization to the job details view. It implements a self-contained graph component using React Flow and dagre that walks job dependencies bidirectionally with bounded depth, resolves job identifiers, fetches edges via silent API calls, builds hierarchical nodes for jobs/layers/frames, and renders an interactive layout. Visibility state is managed via a shared React hook with cross-tab synchronization, and toggles are integrated into the header, sidebar, and job details panel. ChangesJob Dependency Graph Feature
Sequence DiagramsequenceDiagram
participant User
participant Header as AppHeader
participant Sidebar as AppSidebar
participant Hook as useShowDependencyGraph
participant LocalStorage
participant JobDetails as JobDetailsInline
participant Graph as JobDependencyGraph
participant API
User->>Header: click "View Job Graph"
Header->>Hook: toggleGraph()
Hook->>LocalStorage: set show=true
Hook->>Header: update state
Hook->>Sidebar: dispatch CustomEvent
Sidebar->>Hook: listener updates
Hook->>JobDetails: update state
JobDetails->>Graph: render graph
Graph->>API: resolve job names, fetch depends
API-->>Graph: nodes and edges
Graph->>Graph: layout with dagre
Graph-->>JobDetails: render React Flow
User->>Graph: click node
Graph->>JobDetails: onNodeNavigate(jobName)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @rajaryan2007 , thank you for your contribution! Please note that we recently made major improvements to CueWeb. Please review the PR merged into master: Please merge the latest master branch into your branch, resolve any conflicts, and update your PR to follow the new CueWeb interface patterns, menus, and UI structure where applicable. Once your changes are ready, please change the PR status from Draft to Open. This will automatically trigger the initial review by CodeRabbit. After that, I will perform a deeper review of the pull request, provide feedback, and request additional changes if needed. Once again, thank you for your contribution and support to OpenCue. Note: I am currently working on the full migration of CueGUI features into CueWeb, so you should expect additional changes to continue being merged into master. Because of that, you may need to periodically sync and merge the latest master updates into your branch during development. |
|
Note: The |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
cueweb/components/ui/job-dependency-graph.tsx (2)
17-34: ⚡ Quick winModule-level
dagreGraphretains stale nodes across layout calls.
dagreGraphis a shared singleton;getLayoutedElementsadds nodes/edges but never removes ones from prior runs. Across re-layouts (job switch, theme toggle) stale nodes accumulate, which can skew positions. Instantiate a fresh graph per call.♻️ Proposed fix
-const dagreGraph = new dagre.graphlib.Graph(); -dagreGraph.setDefaultEdgeLabel(() => ({})); - const nodeWidth = 200; const nodeHeight = 80; const getLayoutedElements = (nodes: Node[], edges: Edge[], direction = 'TB') => { + const dagreGraph = new dagre.graphlib.Graph(); + dagreGraph.setDefaultEdgeLabel(() => ({})); dagreGraph.setGraph({ rankdir: direction });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cueweb/components/ui/job-dependency-graph.tsx` around lines 17 - 34, The bug is that the module-level dagreGraph variable retains nodes between calls; change getLayoutedElements to instantiate a new dagre.graphlib.Graph() inside the function (instead of using the shared dagreGraph), call setDefaultEdgeLabel and setGraph on that local graph, then use that local graph for setNode, setEdge and dagre.layout to ensure each layout call starts with a fresh graph; update/remove references to the module-level dagreGraph so all layout logic uses the local variable in getLayoutedElements.
60-125: ⚡ Quick winAvoid re-fetching dependencies on theme change.
resolvedThemeis in the effect's dependency array, so toggling light/dark triggers a fullgetDependsForJobnetwork round-trip just to recolor nodes. Consider fetching the raw dependencies in an effect keyed only onjob.id, and deriving theme-dependentstyleeither in a separate effect or viauseMemoover the cached dependency data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cueweb/components/ui/job-dependency-graph.tsx` around lines 60 - 125, The effect currently re-fetches dependencies whenever resolvedTheme changes because resolvedTheme is in the dependency array; change this by splitting data fetch and theme-dependent styling: keep the data-loading effect keyed only on job.id (the useEffect that calls loadGraph which uses getDependsForJob and populates rawNodes/rawEdges then calls setNodes/setEdges) so it only fetches once per job, and remove resolvedTheme from that effect’s deps; then derive node/edge styles on theme changes separately (e.g., a useMemo or a second effect that maps over the cached nodes/edges and updates their style based on resolvedTheme) so you only recolor nodes without calling getDependsForJob again.cueweb/app/jobs/columns.tsx (1)
27-27: ⚡ Quick winRemove unused
FramesLayersPopupimport incueweb/app/jobs/columns.tsx
FramesLayersPopupis imported (line 27) but not referenced anywhere in the file; remove the import (or use the component).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cueweb/app/jobs/columns.tsx` at line 27, The import FramesLayersPopup is unused in the module; remove the import line "FramesLayersPopup" from the imports in this file (or if the component was intended to be used, add the appropriate JSX reference to FramesLayersPopup where needed). Update imports so no unused symbol remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cueweb/app/api/job/getdepends/route.ts`:
- Line 37: The code dereferences responseData.data without guarding for missing
data; update the dependsArray assignment to guard the whole chain (e.g., use
responseData?.data?.depends?.depends || []) so that when responseData or
responseData.data is missing the route returns an empty array instead of
throwing; modify the line that defines dependsArray to use optional chaining on
responseData and data (refer to the dependsArray variable and responseData
object).
- Around line 27-31: Remove the redundant JSON stringify/parse round-trip and
guard against malformed/empty bodies by wrapping await request.json() in a
try/catch; parse into jsonBody (e.g., const jsonBody = await request.json())
inside the try, validate that jsonBody is an object (typeof jsonBody ===
'object' && jsonBody !== null'), and on error or invalid body return
NextResponse.json({ error: 'Invalid request body' }, { status: 400 }); update
the code around jsonBody in route.ts to use this validated value.
In `@cueweb/components/ui/job-dependency-graph.tsx`:
- Around line 110-117: The graph state isn't cleared when a fetch returns no
dependencies, because setNodes/setEdges are only invoked inside the
rawNodes.size > 0 branch; update the logic around getLayoutedElements so that
when rawNodes.size === 0 (or the fetch errors) you explicitly clear state by
calling setNodes([]) and setEdges([]) and also ensure you clear nodes/edges
early when job.id changes (before/when starting the fetch) so the previous graph
won't persist; modify the code that uses rawNodes/rawEdges/getLayoutedElements
and the fetch-handling path to call setNodes([])/setEdges([]) on empty results
or failure.
---
Nitpick comments:
In `@cueweb/app/jobs/columns.tsx`:
- Line 27: The import FramesLayersPopup is unused in the module; remove the
import line "FramesLayersPopup" from the imports in this file (or if the
component was intended to be used, add the appropriate JSX reference to
FramesLayersPopup where needed). Update imports so no unused symbol remains.
In `@cueweb/components/ui/job-dependency-graph.tsx`:
- Around line 17-34: The bug is that the module-level dagreGraph variable
retains nodes between calls; change getLayoutedElements to instantiate a new
dagre.graphlib.Graph() inside the function (instead of using the shared
dagreGraph), call setDefaultEdgeLabel and setGraph on that local graph, then use
that local graph for setNode, setEdge and dagre.layout to ensure each layout
call starts with a fresh graph; update/remove references to the module-level
dagreGraph so all layout logic uses the local variable in getLayoutedElements.
- Around line 60-125: The effect currently re-fetches dependencies whenever
resolvedTheme changes because resolvedTheme is in the dependency array; change
this by splitting data fetch and theme-dependent styling: keep the data-loading
effect keyed only on job.id (the useEffect that calls loadGraph which uses
getDependsForJob and populates rawNodes/rawEdges then calls setNodes/setEdges)
so it only fetches once per job, and remove resolvedTheme from that effect’s
deps; then derive node/edge styles on theme changes separately (e.g., a useMemo
or a second effect that maps over the cached nodes/edges and updates their style
based on resolvedTheme) so you only recolor nodes without calling
getDependsForJob again.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d139248-e52f-40fb-8342-4048f85b0cc8
⛔ Files ignored due to path filters (1)
cueweb/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
cueweb/app/api/job/getdepends/route.tscueweb/app/jobs/columns.tsxcueweb/app/utils/get_utils.tscueweb/components/ui/frames-layers-popup.tsxcueweb/components/ui/job-dependency-graph.tsxcueweb/components/ui/job-details-inline.tsxcueweb/package.json
|
Thanks for your contributions to the OpenCue project. I am reviewing your code and this PR. |
…View Job Graph toggle - BFS walk now resolves job names to UUIDs via GetJobs regex before hitting GetDepends/GetWhatDependsOnThis (cuebot rejects name-only, so the original implementation crashed with "Job not found" on every hop). All BFS fetches go through a silent helper that bypasses accessGetApi so misses don't cascade into red toasts. - Killed the module-level dagre.Graph singleton; layoutNodes() now allocates a fresh graph per call so node/edge state doesn't leak across job switches. - Split the single useEffect into data-fetch (keyed on job.id) and cursor-style (keyed on resolvedTheme) so dark/light toggling no longer re-fetches every depend in the tree. - Replaced the default react-flow node with a DependencyNode that truncates long names, exposes the full name as a title tooltip, uses kind-coded left borders (JOB/LAYER/FRAME), and rings the focus job. Hierarchical labels for layer/frame nodes keep their job context. - Scoped the react-flow pane cursor via a per-instance data-graph-id attribute so two graphs on the page don't share a global style; SVG data-URL memoized per theme. - Clicking a node now navigates to /jobs/<name>?tab=overview instead of firing the misleading "Navigating to..." toast. - Deleted the duplicate /api/job/getdepends route. getDependsForJob takes a Job (matching getLayersForJob / getFramesForJob) and routes through the validated /api/job/action/getdepends endpoint that the rest of CueWeb already uses. - Deleted frames-layers-popup.tsx and its FramesLayersPopup trigger in JobDetailsInline (plus the dead import in columns.tsx). The popup duplicated the Layers + Frames tables JobDetailsInline already renders inline. - The graph now mounts as a third stacked panel inside JobDetailsInline (under the Frames table), matching CueGUI's JobMonitorGraph dock layout. Panel header has a show/hide toggle and a close button. - New Cuetopia > View Job Graph entry in the header dropdown and the sidebar (expanded + collapsed). Checkable item with the same Check icon pattern as File > Disable Job Interaction. Menu, panel, and other tabs stay in sync via a new useShowDependencyGraph hook (localStorage + CustomEvent + storage event) - replaces the brittle ?graph=1 URL param that couldn't re-open the panel after closing because the URL hadn't changed.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cueweb/components/ui/app-sidebar.tsx`:
- Around line 499-505: The button toggle that uses role="switch" (the element
that calls toggleGraph and reads showGraph) should not also expose aria-pressed;
remove the aria-pressed={showGraph} attribute so the control uses a single ARIA
state model (role="switch" with aria-checked={showGraph}) to match the expanded
version and avoid conflicting semantics.
In `@cueweb/components/ui/job-dependency-graph.tsx`:
- Around line 85-102: The responses returned by silentGetDepends and
silentGetWhatDependsOnThis may contain snake_case keys (e.g., depend_er_job)
while downstream code (ingestDepend in the dependency walker /
view-dependencies-dialog logic) expects camelCase (dependErJob); update
silentGetDepends and silentGetWhatDependsOnThis to normalize each Depend row to
the camelCase shape before returning (map over seq and copy/rename depend_er_job
-> dependErJob, any other snake_case aliases used by the API to their camelCase
counterparts), so the graph-building and ingestDepend callers always receive the
canonical field names regardless of gateway build.
In `@cueweb/components/ui/job-details-inline.tsx`:
- Around line 305-325: JobDependencyGraph is rendered without the onNodeNavigate
prop so node clicks fall back to router.push and leave the current Layers &
Frames detail view; update the JobDependencyGraph usage to pass an
onNodeNavigate callback that either selects the clicked job in-place (e.g., call
the parent selection handler with the job id/name) or routes to the
layers/frames tab for that job. Locate the JobDependencyGraph component
instantiation in job-details-inline.tsx and add a prop onNodeNavigate={(node) =>
{ /* select job or navigate to ?tab=layers or ?tab=frames for node.id */ }},
ensuring you use the same job selection/navigation functions used elsewhere in
this component to preserve in-place selection behavior instead of the global
router.push fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca1dbe93-3b18-4ec4-a155-2a250288d953
📒 Files selected for processing (7)
cueweb/app/utils/get_utils.tscueweb/app/utils/use_show_dependency_graph.tscueweb/components/ui/app-header.tsxcueweb/components/ui/app-sidebar.tsxcueweb/components/ui/frames-layers-popup.tsxcueweb/components/ui/job-dependency-graph.tsxcueweb/components/ui/job-details-inline.tsx
💤 Files with no reviewable changes (1)
- cueweb/components/ui/frames-layers-popup.tsx
Previous version from @rajaryan2007
New version from @ramonfigueiredoWhat's different in this revision?See commit: [cueweb] Add job dependency graph: inline panel + Cuetopia toggle
@ramonfigueiredo changes:I picked up the previous version on top of master (now that #2386 is merged) and refactored it based on the review comments previously provided. Key differences from @rajaryan2007's implementation:
Result: The graph now displays the complete dependency tree, avoids unnecessary error notifications, integrates cleanly into the existing job details layout, and provides a consistent menu-toggle experience. |
50a6044
into
AcademySoftwareFoundation:master
|
Thanks for your contributions to the OpenCue project, @rajaryan2007 Excellent job! PR merged to the master 🎉 |
…pia View Job Graph toggle) (#2392) ## Related Issues - #2310 ## Summarize your change. Documents the read-only, interactive job dependency graph added in #2377 across all six CueWeb docs, plus six screenshots (light + dark). - user-guides: new "Job dependency graph" section (enable via Cuetopia > View Job Graph, where it mounts, reading/navigating nodes, empty state), a Core Features bullet, and the Cuetopia menu toggle note. - other-guides: UI walkthrough. - quick-starts: Job Dependency Graph bullet with a screenshot. - tutorials: "Visualizing the dependency graph" step-by-step subsection. - reference: "Job dependency graph panel" - component, useShowDependencyGraph toggle/persistence, BFS tree walk, name->UUID resolution, silent fetches, node/edge rendering, theme-awareness, empty/loading states. - developer-guide: JobDependencyGraph component, updated JobDetailsInline, useShowDependencyGraph hook, cueweb:show-dependency-graph-changed event row, and a "Dependency graph panel" subsection (new @xyflow/react, dagre, @types/dagre deps). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Documented the new interactive Job Dependency Graph feature, including how to enable it via the Cuetopia menu toggle, visualize bidirectional job dependencies, navigate the graph with pan/zoom controls, and click nodes to open related job details. Added comprehensive guides covering feature overview, quick-start, tutorials, user guide, and reference documentation with screenshots. <!-- end of auto-generated comment: release notes by coderabbit.ai -->





Related Issues
Summarize your change.
Adds a read-only, interactive node-graph view of a job's dependency tree to Cuetopia > Monitor Jobs.
LLM usage disclosure
Raj Aryan: rajaryan2007
Co-authored-by: Raj Aryan rajaryan150149@gmail.com
Co-authored-by: Ramon Figueiredo rfigueiredo@imageworks.com